Skip to content

Conversation

LunaBorowska
Copy link
Contributor

@LunaBorowska LunaBorowska commented Dec 5, 2018

Previous implementation wasn't correct, as an inner iterator could have had side effects. Noticed by @bluss in #56534.

@rust-highfive
Copy link
Contributor

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 5, 2018
@LunaBorowska
Copy link
Contributor Author

Oops, modified this in wrong place, let me fix that.

Previous implementation wasn't correct, as an inner iterator
could have had side effects.
@bluss
Copy link
Contributor

bluss commented Dec 5, 2018

Playground snippet to show the bug, that it silences sideeffects in map.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=9d308367741f5ca7cb80ee50ae9a8168

I think we want to fix this because we don't want our use of specialization to be user-visible or affect sideeffectfulness of iterations.

@LunaBorowska
Copy link
Contributor Author

I will try adding a test case for this bug when I get home.

@bluss
Copy link
Contributor

bluss commented Dec 5, 2018

@bors r+ rollup

thanks!

@bors
Copy link
Collaborator

bors commented Dec 5, 2018

📌 Commit a964307 has been approved by bluss

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Dec 5, 2018
Use inner iterator may_have_side_effect for Cloned

Previous implementation wasn't correct, as an inner iterator could have had side effects. Noticed by @bluss in rust-lang#56534.
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Dec 5, 2018
Use inner iterator may_have_side_effect for Cloned

Previous implementation wasn't correct, as an inner iterator could have had side effects. Noticed by @bluss in rust-lang#56534.
bors added a commit that referenced this pull request Dec 5, 2018
Rollup of 15 pull requests

Successful merges:

 - #51753 (Document `From` implementations)
 - #55563 (Improve no result found sentence in doc search)
 - #55987 (Add Weak.ptr_eq)
 - #56119 (Utilize `?` instead of `return None`.)
 - #56372 (Refer to the second borrow as the "second borrow" in E0501.rs)
 - #56388 (More MIR borrow check cleanup)
 - #56424 (Mention raw-ident syntax)
 - #56452 (Remove redundant clones)
 - #56456 (Handle existential types in dead code analysis)
 - #56466 (data_structures: remove tuple_slice)
 - #56476 (Fix invalid line number match)
 - #56497 (cleanup: remove static lifetimes from consts in libstd)
 - #56498 (Fix line numbers display)
 - #56523 (Added a bare-bones eslint config (removing jslint))
 - #56538 (Use inner iterator may_have_side_effect for Cloned)

Failed merges:

r? @ghost
@bors bors merged commit a964307 into rust-lang:master Dec 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants